@s7habo can you look at this and provide your thoughts?

I fall into the use the presets "class", just using presets and occasionally modifying iterations to increase the effect. So, I'll try the new naming and see what disasters I can create 😁

@tgz-code : Please do not merge master into your topic branch. The policy is to rebase onto master when needed. TIA.

Did a brief trial. It segfaulted when returning to lighttable while unloading modules from the view.

Don't rename the module. It breaks EVERY user's shortcuts and automations while serving no purpose. Also every training tool (documentation, video, forum thread) now has the "wrong name" and is thus no longer applicable.

Each time I open an image in darkroom and open DorS, when I return to lighttable the image disappears. I'm guessing renaming the module broke the history and the processing, therefore a new thumbnail can't be built.

If only the UI changed then processing shouldn't be broken.

Did a brief trial. It segfaulted when returning to lighttable while unloading modules from the view.

Don't rename the module. It breaks EVERY user's shortcuts and automations while serving no purpose. Also every training tool (documentation, video, forum thread) now has the "wrong name" and is thus no longer applicable.

Each time I open an image in darkroom and open DorS, when I return to lighttable the image disappears. I'm guessing renaming the module broke the history and the processing, therefore a new thumbnail can't be built.

If only the UI changed then processing shouldn't be broken.

I'm so sorry, I meant to revert the name change before submitting the PR. I will change and update ASAP

Sorry, that master merge was from a commit that I clhad made that I had not pushed (along with the name reversion).

I appreciate you looking at it.

Renaming it back seems to have fixed the disappearing image from lighttable and the crash.

I'm not sure hiding sections is of any use. Almost all of the presets use nearly all of the sliders. Some presets use the "fine tuning" sliders only, which are hidden by default. I think I'd just leave all the sliders visible separated into groups. Imagine that you apply a preset that uses the fine tuning sliders which are hidden, so you see no change to the module and think the preset didn't do anything.

Maybe change edge avoidance to edge following (more <=> less)

Maybe take the Coarse and Fine settings at the top and treat them like the Auxiliary settings only call them Main. In other words treat both the same whether it's splitting them into Coarse and Fine settings or stacking them together under Main and Auxiliary

Overall I think it's an improvement. I at least have a clue when I'm moving sliders and was able to approach it with an idea in mind and execute it. This is a module that requires a lot of playing with to really understand, but at least now it's approachable.

I give my feedback as a user, not a developer, but one who has watched AP's videos, spent a lot of time playing with each slider, and created my own presets. I am also a native English speaker.

Firstly, I commend the intention! I agree many slider names do not currently convey the meaning, and this PR goes a long way to addressing that.

However, I have some alternative names to propose:

For example, the effect of the 2nd and 4th speed/anisotropy sliders is indistinguishable from the effect of the 1st and 3rd speed/anisotropy sliders.

Although they can actually be distinguished, pretty extreme values need to be used to see the differences, so I agree that moving the 2nd/4th orders to a collapsible section is a good move for simplicity.

What I dislike is that sliders in the "auxiliary" section have basically the same names as those outside it, which only makes things more confusing, not less. Is a user supposed to know how "sharpen <> diffuse" in the "coarse" section, differs from "sharpen <> diffuse (coarse)" in the "auxiliary" section, just by reading the name?

I can think of two possible solutions (where I have replaced "x order" with "low/high frequency"):

a)

sharpen <> diffuse (low frequency):
coarse details
fine details

edge avoidance (low frequency):
coarse details
fine detals

auxiliary controls:

sharpen <> diffuse (high frequency):
coarse details
fine details

edge avoidance (high frequency):
coarse details
fine details

b)

coarse (low frequency):
sharpen <> diffuse
edge avoidance

fine (low frequency):
sharpen <> diffuse
edge avoidance

auxiliary controls:

coarse (high frequency):
sharpen <> diffuse
edge avoidance

fine (high frequency):
sharpen <> diffuse
edge avoidance

*Either of the above solutions also solves the problem of having the same name in both the heading and sliders underneath. (Eg. "Auxiliary sharpen <> diffuse" followed by "sharpen <> diffuse (coarse)". "sharpen <> diffuse" becomes redundant there, being used twice. My solution does not do that.

My understanding of the radius is that "central radius" means values lower than this will not be affected, while "radius span" means values higher than this will not be affected. It sets the inner and outer limits for the effect. From the manual:

The span of diffusion defines a range of detail scales (between center - span and center + span) within which the diffusion is confined.

To reflect this, better names would be:

central radius = inner radius / inner limit / inner range / scale minimum
radius span = outer radius / outer limit / outer range / scale maximum

EDIT:
However if the centre is bigger than the span then the min/max or inner/outer gets reversed, so the orinal names might be best after all. Since "central radius" is just another name for "radius" then the top slider could be simplified to that, and the bottom slider could be either "span" or "scale" (which is the opposite to how they are proposed in this PR, but without checking the math, I think more accurate based on the original names). However, if both controls are a radius, then the best alternatives I have are "radius base/core" and "radius span".

*I don't think saying it "scales up the effect" is quite accurate; rather, that it defines one boundary of the range the effect works on.

If a user accidentally increases the effect radius anywhere near the 512 limit and then increases iterations anywhere close to the 128 limit, their computer might just melt :)

Both "dehaze" and "local contrast" presets use values as big as 512 px, which is necessary to achieve the effect; therefore, it is not a good idea to decrease these soft limits.

Some presets also have "anisotropy" values > 100 px (largest I saw was 1000 px), so one could make an argument these soft limits should also be kept; however in my testing, values that high are not nearly as vital to achieve the effect (or even as desirable), so you can make an argument to reduce them. (I half seem to recall that the initial DorS shipped with a bug affecting "anisotropy", which may have influenced some of those preset values, but I've no idea where I heard that).

I am not a big fan of hiding the "edge management/fine-tuning" controls in a drop drown, since they are also integral to the effect. See for example the three local contrast presets, which have "edge sensitivity" as low as 0 and as high as 4.

Also, the last control section, "diffusion spatiality", is a prime example of poor/no abstraction, as this control is primarily intended to help with highlight inpainting, yet the name provides no indication of this.

Actually, that's not totally true. There is a preset named "inpaint highlights" for exactly that purpose.

I renamed the "diffusion spatiality" control section to "highlight inpainting" and made it collapsible.

I would simply rename it "inpainting". Why? Because if the "luminance threshold" is set to 0, it will inpaint the whole image, not just the highlights. That seems to be exactly what the "luminance threshold" slider is for, determining which range it inpaints.

Given it only has one slider, I would also not bother putting the "inpainting" in its own separate drop down. Not worth the extra layer of obfuscation to save just one line of space.

@wpferguson

I'm not sure hiding sections is of any use. Almost all of the presets use nearly all of the sliders. Some presets use the "fine tuning" sliders only, which are hidden by default. I think I'd just leave all the sliders visible separated into groups. Imagine that you apply a preset that uses the fine tuning sliders which are hidden, so you see no change to the module and think the preset didn't do anything.

I think the main issue I am trying to solve is that the module does not feel approachable to users. Part of the issue is that there are fifteen (15) sliders in the module that are all visible. I think hiding non-essential controls by default will greatly improve this. At least from my experience, when I first try a new module, I do so in an exploratory way, instead of immediately going through presets, so this simplified view, which is meant for simple testing/exploration, seems like a more intuitive interface.

As for selecting a preset and then not seeing some sliders that have been modified, I don't know how many users really mess with settings after applying presets (other than iterations, radius...).

I have thought about having the fine-tuning panel open by default since they are so useful, but I am also trying to lean into simplicity. We have had a complex version of DorS for a while :)

Currently, with the sliders that are visible by default in my version, you can easily create sharpen/local contrast/bloom effects quite quickly, without needing to access any hidden sliders (You do need parametric masking for bloom). Some of the hidden sliders are useful, but they are generally not essential to any effect (except the luminance threshold under highlight inpainting).

@SoupyGit

What I dislike is that sliders in the "auxiliary" section have basically the same names as those outside it, which only makes things more confusing, not less. Is a user supposed to know how "sharpen <> diffuse" in the "coarse" section, differs from "sharpen <> diffuse (coarse)" in the "auxiliary" section, just by reading the name?

Since, in my opinion, it is incredibly difficult to see a real difference between 1st/3rd sliders and 2nd/4th order sliders, I am not sure that it requires any distinction beyond "auxiliary" or a similar word. Now, maybe there is a valid reason to use one over the other, but I feel like it is such a narrow use case that it could simply lead to more confusion if we try to express that narrow use case in the UI. For example, currently there is a distinction between them via the "nth order.." prefixes, but that only adds confusion when people realize that 3rd and 4th order controls can be used to basically negate the effects of 1st/3rd order controls if you set them to inverse values. I know that I was confused by this when I first began using the module.

Also, I apologize, but the example alternative layouts don't make sense to me because they mark all auxiliary controls as "(high frequencies)", which is not correct as far as I can tell. 3rd order speed affects low-frequency wavelet layers, and 4th order speed parameters affect high-frequency wavelet layers, according to the tooltips. Maybe that was just a typo on your part or something. Let me know.

My understanding of the radius is that "central radius" means values lower than this will not be affected, while "radius span" means values higher than this will not be affected. It sets the inner and outer limits for the effect. From the manual:

The span of diffusion defines a range of detail scales (between center - span and center + span) within which the diffusion is confined.

To reflect this, better names would be:

central radius = inner radius / inner limit / inner range / scale minimum radius span = outer radius / outer limit / outer range / scale maximum

EDIT: However if the centre is bigger than the span than the min/max or inner/outer gets reversed, so the orinal names might be best after all (or "radius centre" and "radius span")

*I don't think saying it "scales up the effect" is quite accurate; rather, that it defines one boundary of the range the effect works on.

After some additional testing, I think that my logic still holds, but that what I mean by the names I picked might need clarifying. Yes, it is true that central radius defines the main scale that the diffusion algorithm works on, and the radius span defines another larger circle with radius = central radius+radius span (roughly). If you can imagine that the diffusion algorithm only worked on a single spot, everything inside the central radius would be left untouched, and everything between the central radius and the outer circle would be diffused/sharpened. This is why, if you increase the central radius to be bigger than the radius span, you get overall more untouched detail than you do diffused/sharpened detail.

Another example of where this behavior comes into play is when dealing with images of varying resolution. A smaller MP image might require a central radius of 0 and a radius span of 32 for a nice diffusion effect, but on a larger image, you can effectively "scale" up the module's effect in order to accomplish the same effect in relation to the much higher pixel count (e.g. double central radius and radius span in order to counteract the effect of higher resolution).

You can see the "scaling" effect of the central radius control by heavily sharpening an image with geometric lines, and then increasing the central radius. The sharpening actually starts to look zoomed towards the screen, or like you are removing the influence of small details from the final effect. The distance/radius of sharpening or diffusion stays the same (since radius span is not changed), but the range of detail sizes that contribute to that diffusion/sharpening gets filtered to larger and larger structures.

Here is the same sharpening effect, each with a different central radius: 0, 8, 16, 32. You can see that in the first image, the sharpening is well distributed and proportional across the image, and then by the time you get to the final image, only very large edges are contributing to the sharpening and the sharpening lines are quite thick. But because only larger details are contributing to the effect, many smaller details like windows in the building are left mostly unaffected.

If you are interested, here is the same "scaling up" effect with diffusion. Again with central radius: 0,8,16,32:

If a user accidentally increases the effect radius anywhere near the 512 limit and then increases iterations anywhere close to the 128 limit, their computer might just melt :)

Both "dehaze" and "local contrast" presets use values as big as 512 px, which is necessary to achieve the effect; therefore, it is not a good idea to decrease these soft limits.

Some presets also have "anisotropy" values > 100 px (largest I saw was 1000 px), so one could make an argument these soft limits should also be kept; however in my testing, values that high are not nearly as vital to achieve the effect (or even as desirable), so you can make an argument to reduce them. (I half seem to recall that the initial DorS shipped with a bug affecting "anisotropy", which may have influenced some of those preset values, but I've no idea where I heard that).

Yes, I know that some presets require those higher values, but the presets still work with the soft limits adjusted. I could certainly increase the soft limits, but I do think that for general purpose use cases, the tighter ranges make the module easier to use and maybe more "fun" due to faster feedback and less possible options to explore.

I am not a big fan of hiding the "edge management/fine-tuning" controls in a drop drown, since they are also integral to the effect. See for example the three local contrast presets, which have "edge sensitivity" as low as 0 and as high as 4.

Yeah, maybe I should have fine tuning expanded by default? The thing is, that usually you can achieve a reduction of edge artifacts (which edge sensitivity is used for), by simply reducing some of the sharpening or decreasing radius span... so even though those presets might use extra edge sensitivity, it might not be entirely necessary. But it is definitely worth considering.

Also, the last control section, "diffusion spatiality", is a prime example of poor/no abstraction, as this control is primarily intended to help with highlight inpainting, yet the name provides no indication of this.

Actually, that's not totally true. There is a preset named "inpaint highlights" for exactly that purpose.

I mean that the name of the control and its section title do not hint at the fact that it is primarily meant for highlight inpainting. You have to read the tooltip or go through presets to figure that out. Since highlight inpainting is sort of a rare need, I figured naming the section clearly would help keep people from feeling like they need to look into it ("diffusion spatiality" is an awefully exciting sci-fi sounding name :) ).

I renamed the "diffusion spatiality" control section to "highlight inpainting" and made it collapsible.

I would simply rename it "inpainting". Why? Because if the "luminance threshold" is set to 0, it will inpaint the whole image, not just the highlights. That seems to be exactly what the "luminance threshold" slider is for, determining which range it inpaints.

Given it only has one slider, I would also not bother putting the "inpainting" in its own separate drop down. Not worth the extra layer of obfuscation to save just one line of space.

Setting the luminance threshold to 0, actually just deactivates all inpainting. So technically, it always paints the "highlights", even if "highlights" might refer to 99% of the image.
Also, in reference to it only being one slider, I put it in it's own section because it is already in its own section and making it collapsible save room while also not having an awkward lone control dangling at the bottom of the module :)

For example, currently there is a distinction between them via the "nth order.." prefixes, but that only adds confusion when people realize that 3rd and 4th order controls can be used to basically negate the effects of 1st/3rd order controls if you set them to inverse values.

I agree, that's confusing. But it is even more confusing to have two different sliders basically named the same thing. Users will have no idea what makes them different.

Also, I apologize, but the example alternative layouts don't make sense to me because they mark all auxiliary controls as "(high frequencies)", which is not correct as far as I can tell.

This gets pretty confusing. Let me explain my reasoning:

From the manual (simplified):

1st/2nd order speed:
low freq wavelet relative to 1st/2nd order anisotropy

3rd/4th order speed:
high freq wavelet relative to 3rd/4th order anisotropy.

1st order anisotropy:
low freq wavelet relative to low freq gradient

2nd order anisotropy:
low freq wavelet relative to high freq gradient

3rd order anisotropy:
high freq wavelet relative to low freq gradient

4th order anisotropy:
high freq wavelet relative to high freq gradient

So, putting it all together:

speed = diffuse <> sharpen
direction = edge avoidance
1st/2nd order speed = fine detail
3rd/4th order speed = coarse detail
1st order anisotropy = low/low freq
2nd order anisotropy = low/ high freq
3rd order anisotropy = high/low freq
4th order anisotrophy = high/high freq.

How to communicate all that complexity in simple (yet also unique) names?

fine (1st order)
s <> d
edge avoidance

coarse (3rd order)
s <> d
edge avoidance

auxiliary controls:

fine (2nd order)
s <> d
edge avoidance

coarse (4th order)
s <> d
edge avoidance

I previously used "low and high frequency" instead of "x order", though I may not have done so correctly. The main point is to find a way to differentiate the names, in a way that is both consistent and informative. Tricky.

A smaller MP image might require a central radius of 0 and a radius span of 32 for a nice diffusion effect, but on a larger image, you can effectively "scale" up the module's effect in order to accomplish the same effect in relation to the much higher pixel count

I understand the effect you are describing, but it's important the name does not contradict the underlining math. That's not my forte, but others should definitely look into it. The central radius will not always be smaller than the radius span (see for instance "local contrast normal" preset). In that case, increasing the central radius further will actually flatten the contrast, not enhance it.

the tighter ranges make the module easier to use and maybe more "fun" due to faster feedback and less possible options to explore.

It would not be more "fun" for anyone wanting to use the module for local contrast, since they'd have to right click to extend the soft limits. I think you are limiting the scope of the module too much.

Yeah, maybe I should have fine tuning expanded by default?

Wouldn't that negate the benefit of drop down menus, which is to save vertical space? IMO you either have it or you don't. I vote you don't.

The thing is, that usually you can achieve a reduction of edge artifacts (which edge sensitivity is used for), by simply reducing some of the sharpening or decreasing radius span.

Sure, but by limiting yourself to fewer sliders you are limiting the potential of the module. I don't think 'chroma' in the color balance rgb module would be put in a drop down just because 'saturation' also exists.

Since highlight inpainting is sort of a rare need, I figured naming the section clearly would help keep people from feeling like they need to look into it.

I think "inpainting" on its own achieves this just fine.

Setting the luminance threshold to 0, actually just deactivates all inpainting.

Hm, I tested settings where it produced a solid colour image; but regardless, start with the "inpaint highlights" preset and set "luminance threshold" to a very low value and see how it inpaints a lot more than just the highlights.

To me, the difference between naming it "highlight inpainting" or "inpainting" is very minor, since most people will not touch it ever, but just thought I'd explain my reasoning. The naming of the other sliders matters a lot more.

From the manual (simplified):

1st/2nd order speed: low freq wavelet relative to 1st/2nd order anisotropy

3rd/4th order speed: high freq wavelet relative to 3rd/4th order anisotropy.

1st order anisotropy: low freq wavelet relative to low freq gradient

2nd order anisotropy: low freq wavelet relative to high freq gradient

3rd order anisotropy: high freq wavelet relative to low freq gradient

4th order anisotropy: high freq wavelet relative to high freq gradient

So, putting it all together:

speed = diffuse <> sharpen direction = edge avoidance 1st/2nd order speed = fine detail 3rd/4th order speed = coarse detail 1st order anisotropy = low/low freq 2nd order anisotropy = low/ high freq 3rd order anisotropy = high/low freq 4th order anisotrophy = high/high freq.

How to communicate all that complexity in simple (yet also unique) names?

fine (1st order) s <> d edge avoidance

coarse (3rd order) s <> d edge avoidance

auxiliary controls:

fine (2nd order) s <> d edge avoidance

coarse (4th order) s <> d edge avoidance

I think I understand what you are talking about. The anisotropy controls for 2nd and 4th order speeds are using the high-frequency gradient to decide what an "edge" is when setting the direction of diffusion. Likewise the 1st and 3rd order anisotropy controls use the low-frequency gradient to decide what an "edge" is when setting the direction of diffusion.

I think the most important thing here, is not technical accuracy, but effective accuracy. I will do some additional testing to try and understand how this difference plays out (i.e. how it actually changes the module's effect). If you know how this detail affects the output, please let me know.

The central radius will not always be smaller than the radius span (see for instance "local contrast normal" preset). In that case, increasing the central radius further will actually flatten the contrast, not enhance it.

You are correct, the central radius will not always be smaller than the radius span, but even if it is larger, the effect does not get inverted. Apply the local contrast(normal) preset, and then toggle it on and off, and you will see that it does increase the local contrast of the image. The important distinction is that increasing central radius does not always increase the effect, but it does scale the effect up, meaning the effect operates within the realm of larger details instead of fine details. You see the same effect in the screenshots I shared, where I increased central radius to be bigger than the radius span. It doesn't invert the effect, but rather scales the effect up (Sorry if that is not the right way to describe it.

Maybe I could rename it to be something like "base scale radius", to better capture the fact that it is the main scale of the diffusion algorithm (per the tooltip and manual). Or we could just call it "main scale".

It would not be more "fun" for anyone wanting to use the module for local contrast, since they'd have to right click to extend the soft limits. I think you are limiting the scope of the module too much.

Since there are only two presets that use a central radius over 8, and both are just alternate versions of the same effect (local contrast), I don't know that increasing the soft range beyond my current soft limit of 64 is really necessary. I know AP is smart, but since these are pixel resolutions we are talking about, the value of 500+ is not even set in stone as a necessity. Any image with a resolution above the image AP had in mind would technically need right-click operation anyways to increase the central radius beyond 512. While it is nice to not have to right click, it is also annoying to try and fine tune a control by dragging when the range is so large.

To provide a counter point to the "necessity" of high central radius values. Here is an example with an image on the left having the regular local contrast (normal) preset applied to it, and then on the right, the same preset but with central radius and radius span values that are far smaller. I have also reduced the iterations. Both images look nearly identical.

So no, I don't think the super high range is really needed, especially in the case of local contrast.

Yeah, maybe I should have fine tuning expanded by default?

Wouldn't that negate the benefit of drop down menus, which is to save vertical space? IMO you either have it or you don't. I vote you don't.

The thing is, that usually you can achieve a reduction of edge artifacts (which edge sensitivity is used for), by simply reducing some of the sharpening or decreasing radius span.

Sure, but by limiting yourself to fewer sliders you are limiting the potential of the module. I don't think 'chroma' in the color balance rgb module would be put in a drop down just because 'saturation' also exists.

I will switch to having it be a regular control section and see how that looks.

Since highlight inpainting is sort of a rare need, I figured naming the section clearly would help keep people from feeling like they need to look into it.

I think "inpainting" on its own achieves this just fine.

Setting the luminance threshold to 0, actually just deactivates all inpainting.

Hm, I tested settings where it produced a solid colour image; but regardless, start with the "inpaint highlights" preset and set "luminance threshold" to a very low value and see how it inpaints a lot more than just the highlights.

To me, the difference between naming it "highlight inpainting" or "inpainting" is very minor, since most people will not touch it ever, but just thought I'd explain my reasoning. The naming of the other sliders matters a lot more.

I will change to just "inpainting"

I‘m not convinced, that replacing technical terms needed to understand the physical diffusion process based nature of the module improves something.
A complex module can‘t be made easier to use by beginners (and that seems to be the problem to be solved) if we label known terminology in it.
Maybe beginners can get easier a proper result, but never will learn about the capabilities of this module since terminology is completely disconnected from the underlying functions.

in my opinion it’s better to invest time in improving tooltips.

if it’s just giving beginners a simple to use tool, then better let them use different modules, which behaves like known tools ;)

100% Agree with Martin. These changes are detrimental to dt. Instead of explaining what the module actually does, it tries to define how someone feels it works.

I think the OP could do a video as a tutorial on how he uses the module without the need to rename the sliders.